Update JmsIO to ActiveMQ 6.2.5 and jakarta.jms#38729
Conversation
Bump activemq to 6.2.5 and qpid-jms-client to 2.10.0, both of which target jakarta.jms. Replace the geronimo-jms_2.0_spec dependency with jakarta.jms-api 3.1.0 and migrate all javax.jms imports and javadoc references in JmsIO main and test sources to jakarta.jms.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the JmsIO module by migrating from the legacy javax.jms namespace to the Jakarta Messaging API (jakarta.jms). This change involves updating core dependencies to versions that support the Jakarta EE standard, ensuring compatibility with modern messaging infrastructure. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the JMS IO module from the legacy javax.jms namespace to the modern jakarta.jms namespace, upgrading ActiveMQ to 6.2.5, Qpid JMS Client to 2.10.0, and adopting jakarta.jms-api:3.1.0. The review feedback identifies critical Java compatibility issues with these upgrades: ActiveMQ 6.x requires Java 17, while Qpid JMS Client 2.x and Jakarta JMS API 3.1.0 require Java 11, which would break Apache Beam's Java 8 and Java 11 compatibility. The reviewer suggests downgrading the Jakarta JMS API to 3.0.0 to maintain Java 8 support and requests verification on how compatibility will be maintained for the other dependencies.
| // There are a few versions are determined by the BOMs by running scripts/tools/bomupgrader.py | ||
| // marked as [bomupgrader]. See the documentation of that script for detail. | ||
| def activemq_version = "5.19.2" | ||
| def activemq_version = "6.2.5" |
There was a problem hiding this comment.
ActiveMQ 6.x requires Java 17 as a minimum runtime version. Since Apache Beam still supports and runs tests on Java 8 and Java 11, upgrading activemq_version to 6.2.5 will cause test execution failures (such as UnsupportedClassVersionError) on Java 8 and Java 11 environments.
Please consider restricting the JmsIO tests to only run when the build JDK is 17 or higher, or verify how Java 8/11 compatibility will be maintained for the test suite.
| // TODO(https://github.com/apache/beam/issues/37637): Remove this once the Bom has been updated to at least reach this version | ||
| def bigtable_version = "2.73.1" | ||
| def qpid_jms_client_version = "0.61.0" | ||
| def qpid_jms_client_version = "2.10.0" |
There was a problem hiding this comment.
Qpid JMS Client 2.x requires Java 11 as a minimum runtime version. Since Apache Beam still supports Java 8, upgrading qpid_jms_client_version to 2.10.0 will break Java 8 compatibility for any users or tests utilizing this client. Please verify if this is intentional or if we need to restrict this dependency/module to Java 11+.
| implementation library.java.slf4j_api | ||
| implementation library.java.joda_time | ||
| implementation "org.apache.geronimo.specs:geronimo-jms_2.0_spec:1.0-alpha-2" | ||
| implementation "jakarta.jms:jakarta.jms-api:3.1.0" |
There was a problem hiding this comment.
jakarta.jms:jakarta.jms-api:3.1.0 (Jakarta Messaging 3.1) is compiled for Java 11 (class file version 55.0). Since Apache Beam still supports Java 8, using version 3.1.0 will break Java 8 compatibility for the beam-sdks-java-io-jms module.
To maintain Java 8 compatibility while migrating to the jakarta.jms namespace, please use jakarta.jms:jakarta.jms-api:3.0.0 (Jakarta Messaging 3.0), which is compiled for Java 8 (class file version 52.0).
implementation "jakarta.jms:jakarta.jms-api:3.0.0"
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
1 participant
Bump activemq to 6.2.5 and qpid-jms-client to 2.10.0, both of which target jakarta.jms. Replace the geronimo-jms_2.0_spec dependency with jakarta.jms-api 3.1.0 and migrate all javax.jms imports and javadoc references in JmsIO main and test sources to jakarta.jms.